Fix greedy_best_first #8775
Merged
Fix greedy_best_first #8775
+67
−53
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
algorithms-keeper
bot
added
enhancement
graphs/greedy_best_first.py
Outdated
| @@ -136,22 +153,21 @@ def get_successors(self, parent: Node) -> list[Node]: | |||
| pos_x = parent.pos_x + action[1] | |||
There was a problem hiding this comment.
You could get the successors using a list comprehension:
successors = [
Node(
(pos_x := parent.pos_x + action[1]),
(pos_y := parent.pos_y + action[0]),
self.target.pos_x,
self.target.pos_y,
parent.g_cost + 1,
parent,
)
for action in delta
if (
0 <= pos_x < len(self.grid[0])
and 0 <= pos_y < len(self.grid)
and self.grid[pos_y][pos_x] == 0
)
]but ultimately I could go either way since this isn't necessarily more readable than what you already have.
There was a problem hiding this comment.
@tianyizheng02
thank for your comment. i tried but i got this failed.
ruff.....................................................................Failed
- hook id: ruff
- exit code: 1
graphs/greedy_best_first.py:153:18: F841 Local variable `pos_x` is assigned to but never used
|
151 | return [
152 | Node(
153 | (pos_x := parent.pos_x + action[1]),
| ^^^^^ F841
154 | (pos_y := parent.pos_y + action[0]),
155 | self.target.pos_x,
|
= help: Remove assignment to unused variable `pos_x`
graphs/greedy_best_first.py:154:18: F841 Local variable `pos_y` is assigned to but never used
|
152 | Node(
153 | (pos_x := parent.pos_x + action[1]),
154 | (pos_y := parent.pos_y + action[0]),
| ^^^^^ F841
155 | self.target.pos_x,
156 | self.target.pos_y,
|
= help: Remove assignment to unused variable `pos_y`
Found 2 errors.
black....................................................................Passed
codespell................................................................Passed
pyproject-fmt........................................(no files to check)Skipped
Validate filenames.......................................................Passed
Validate pyproject.toml..............................(no files to check)Skipped
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1
graphs/greedy_best_first.py:162: error: Name "pos_x" is used before definition [used-before-def]
graphs/greedy_best_first.py:163: error: Name "pos_y" is used before definition [used-before-def]
graphs/greedy_best_first.py:164: error: Name "pos_y" is used before definition [used-before-def]
graphs/greedy_best_first.py:164: error: Name "pos_x" is used before definition [used-before-def]
Found 4 errors in 1 file (checked 1 source file)
There was a problem hiding this comment.
Sorry, I think this is the correct syntax?
successors = [
Node(
pos_x,
pos_y,
self.target.pos_x,
self.target.pos_y,
parent.g_cost + 1,
parent,
)
for action in delta
if (
0 <= (pos_x := parent.pos_x + action[1]) < len(self.grid[0])
and 0 <= (pos_y := parent.pos_y + action[0]) < len(self.grid)
and self.grid[pos_y][pos_x] == 0
)
]There was a problem hiding this comment.
@tianyizheng02
Thanks, I applied list comprehension.
- node in self.open_nodes is always better node TheAlgorithms#8770
for more information, see https://pre-commit.ci
JadeKim042386
force-pushed
the
fix_greedy_best_first
branch
from
4ba71ab to
c63a974
Compare
tianyizheng02
approved these changes
Aug 15, 2023
algorithms-keeper
bot
removed
the
awaiting reviews
sedatguzelsemme
pushed a commit
to sedatguzelsemme/Python
that referenced
this pull request
Sep 15, 2024
* fix: typo TheAlgorithms#8770 * refactor: delete unnecessary continue * add test grids * fix: add \_\_eq\_\_ in Node class TheAlgorithms#8770 * fix: delete unnecessary code - node in self.open_nodes is always better node TheAlgorithms#8770 * fix: docstring * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix: docstring max length * refactor: get the successors using a list comprehension * Apply suggestions from code review --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tianyi Zheng <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your change:
fixes: #8770
Checklist:
Fixes: #{$ISSUE_NO}.